Make the roadmap reachable, and stop a phase plan claiming shipped work is proposed (#395) - #407
Conversation
…rk is proposed Three parts, following the plan on commandprompt#395. A user-facing docs/roadmap.md, in the nav under Development. jd went looking for the file listing upcoming features and could not find it, because the only routes to design/ROADMAP.md were a raw GitHub link in docs/index.md and one in the CHANGELOG header. This publishes a new entry-point page rather than moving design/ROADMAP.md into docs/. That document is a working record: it fails the user-doc style rules in 26 places, and it carries internal notes such as an account of four previous rewrites of itself being wrong. Publishing it verbatim would put working notes on the documentation site, which is the class of problem this issue is about. The new page states status, done, planned and under-investigation, and links to the working record for detail. PHASE_G_EXTERNAL_PARQUET_PLAN.md said "Status: proposed, not implemented" for work that has shipped: the reader, the FDW surface, pushdown, multi-file and partition pruning are all in the tree with suites behind them. It now says shipped and points at ROADMAP.md for anything outstanding. The object-storage item moves out of that file's "Open decisions for review" and into ROADMAP.md's Remaining, pointing at commandprompt#393 and commandprompt#394. A breadcrumb stays behind so a reader does not conclude it was dropped. That heading was the trap: a decided item that was also outstanding work, filed under open decisions. Also carries a correction. The original wording said the scan core is unchanged because both paths just hand it bytes. True of the core, and it understates ranged GETs, concurrency, retries and credentials. docs_style gains the narrow check from the plan: fail if the roadmap is not in the nav. A page outside the nav is not published, and nothing else would notice it had gone. Closes commandprompt#395.
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
One blocking inconsistency, then this is good to merge.
The premise is right and the guard is the right shape. I verified it by removal rather
than by reading it.
The guard works, and here is where its edge is
PASS the roadmap is in the documentation nav <- as written
FAIL the roadmap is in the documentation nav <- nav entry deleted
PASS the roadmap is in the documentation nav <- PAGE deleted, nav entry kept
The third line is the one to know about. The check greps mkdocs.yml and never asks
whether docs/roadmap.md exists, so a deleted page with a live nav entry passes it.
Not blocking, because docs.yml runs mkdocs build --strict, which fails on a nav
entry pointing at a missing file. The two together do cover it. Worth one clause in the
comment saying so, since the check reads as if it owns reachability on its own and it
owns half.
Blocking: the new page contradicts the roadmap it links to
docs/roadmap.md lists under Done:
PostgreSQL integration. Read stream and asynchronous IO, virtual generated
columns, temporal constraints, statistics collection for the planner.
design/ROADMAP.md, in this same PR, still says at the bottom:
- Read stream / AIO in the scan -- shipped, see the Done table.
- Virtual generated columns (PostgreSQL 18): confirm read-time generation on a
columnar table and add differential coverage.- Temporal constraints: verify enforcement and add coverage.
Whoever updated the read-stream line to "shipped" left the two below it stale. So the
file's Done table says covered, its adoption list says go and confirm it, and the new
user-facing page picks the first and links to the second.
The Done table is the one telling the truth: test/generated_columns.sh and
test/temporal.sh both exist, and #411 says the same. So the fix is to update those two
lines, not the new page.
I am requesting changes on this rather than waving it through because it is the same
defect this PR exists to fix. A roadmap that claims stale status is what #395 was about,
and shipping a user-facing page whose "Done" list is contradicted two clicks away
reproduces it at the surface where it costs most.
Either update those lines here, or land #411 first and rebase. #411 already rewrites the
neighbourhood, so ordering it first is probably less work.
Smaller notes, none blocking
- The Phase G header now reads
Status: **shipped**and, four lines down,## Open decisions for reviewwith a "Confirm building both surfaces" item. If the surfaces
shipped, that decision is closed. The heading fights the status. - "It was previously recorded only in
PHASE_G_EXTERNAL_PARQUET_PLAN.mdunder a heading
reading 'open decisions', which is why nobody found it" is a good line. Keep it. - "It is not a commitment, and it is not a schedule" is the right disclaimer and the
right length.
…er (commandprompt#395) The roadmap's Done table said PostgreSQL integration was covered while its adoption list four screens down still said go and confirm generated columns and temporal constraints, and go and investigate REPACK. Whoever marked read stream shipped left the three below it stale. The Done table was the one telling the truth. test/generated_columns.sh and test/temporal.sh both exist and run in the matrix, and REPACK was investigated and does not work on a columnar table (commandprompt#399, commandprompt#409). The list now says so. Verified rather than assumed, because the first draft of this commit got it wrong in the same direction: it recorded the all-null chunk for a virtual generated column as an open write-path finding. columnar_write_state.c skips attgenerated == 'v' entirely and generated_columns.sh pins it, so that shipped too, and the bullet now says what the code does. Also, the reachability check owned half its property. It grepped mkdocs.yml and never asked whether docs/roadmap.md existed, so deleting the page while keeping the nav entry passed. mkdocs build --strict catches that in docs.yml, but a property asserted half here and half in a workflow is how the missing half goes unnoticed. Both halves are now stated where the property is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw
Both fixed, and the blocking one caught me getting it wrong the same way.The contradiction is gone, and landing #411 first would not have fixed itYou suggested ordering #411 first since it rewrites that neighbourhood. It does not touch They now read:
My first draft of that fix was wrong in the same directionI wrote the all-null chunk for a virtual generated column into the bullet as an open The nav check owned half its property, so it now owns bothYou were right that a deleted page with a live nav entry passes, and right that check "the roadmap is in the documentation nav" ...
check "and the page that nav entry points at exists" \
"$([ -f "$SRCDIR/docs/roadmap.md" ] && echo yes || echo no)" "yes"The comment states both facts and names While there I found that my own insertion had orphaned the The Phase G headingAgreed on |
jdatcmd
left a comment
jdatcmd
left a comment
There was a problem hiding this comment.
Both addressed, and the reachability fix is better than what I asked for.
I asked for a comment noting the nav check owns half the property. You asserted the other
half instead, which is the right call:
PASS the roadmap is in the documentation nav
PASS and the page that nav entry points at exists
Removal-proved it, deleting the page and keeping the nav entry:
FAIL and the page that nav entry points at exists: got [no] want [yes]
That case passed before. And the comment now says where the other half lives and why both
are stated here rather than split across a workflow, which is the part that stops someone
deleting one as redundant.
The adoption list no longer contradicts the Done table, and it does better than asserting
"done": it names columnar_write_state.c skipping attgenerated == 'v' and the suite
that pins it. That is checkable by the next reader without trusting either of us.
docs_style.sh passes at 5 checks.
One merge-order note, not a change request
design/ROADMAP.md on this branch still points at PG18_19_OPPORTUNITIES.md in two
places, lines 24 and 250, and #411 renames that file to POSTGRESQL_VERSION_ADOPTION.md
and rewrites both lines. So these two will conflict in ROADMAP.md.
Nothing to fix here. Merging this first and rebasing #411 is the smaller job, since #411
already rewrites that neighbourhood. Flagging so the conflict is expected rather than a
surprise.
Merging.
main corrected the REPACK entry in PG18_19_OPPORTUNITIES.md (commandprompt#399, commandprompt#409) while this branch renames that file away, so git saw a modify/delete and the correction would have been dropped on the floor by taking either side whole. The renamed file's rewrite of section 5 is better prose and states the positive case, but it lost the nuance that made the correction worth having: the claim needs TWO fixtures. With no identity index PostgreSQL refuses both access methods before the AM is reached, which is what an early reading mistook for "not columnar-specific"; only with a primary key does heap succeed while columnar raises our error. One fixture reads the wrong way round. That is now back, in the renamed file. Conflict caused by merging commandprompt#407 first, which I predicted in review and said whoever landed second should take. Taking it.
2 participants
Closes #395. Follows your plan, with one deviation I want to flag rather than bury.
1. The roadmap is reachable
New
docs/roadmap.md, in the nav under Development. Status, done, planned with an issueper row, and an under-investigation section for #403, #405 and #390.
The deviation: I did not move
design/ROADMAP.mdintodocs/. You said publishing itas-is versus editing it was a statement about its content, so here is the measurement:
and it contains lines like "Four successive rewrites of this file were each wrong because
they restated numbers that...". That is a working record talking to itself, which is
useful and is not a user-facing page. Publishing it verbatim would put working notes on the
site, which is the class of problem this issue is about. Rewriting 26 sentences would also
be me changing the voice of your document wholesale.
So: a new entry-point page that links to the working record. Say the word if you would
rather I did the edit and moved it, and I will.
2. The misfiling, fixed both ways
The object-storage item moves out of
PHASE_G_EXTERNAL_PARQUET_PLAN.md's "Open decisionsfor review" and into
ROADMAP.md's Remaining, pointing at #393 and #394. A breadcrumbstays so nobody concludes it was dropped.
It also carries a correction. The original said the scan core is unchanged "because both
just hand it bytes". True of the core, and it understates ranged GETs, concurrency, retries
and credentials, which is the substance of #393.
3. The expiry header, and a thing I found while adding it
That file's header read:
External Parquet read is shipped: the reader, the FDW surface, projection and predicate
pushdown, multi-file reads and partition pruning, all with suites behind them. The document
has been describing shipped work as unbuilt.
That is a sharper instance of your point 3 than the one we started with. A stale "not
implemented" is worse than a missing status line, because it reads as authoritative. It now
says shipped and defers to ROADMAP.md for anything outstanding.
I applied it to this one file rather than sweeping all thirty phase plans, because I have
not verified the status of the other twenty-nine and a wrong status header is exactly the
problem. Happy to work through them if you want, but that is a research pass, not an edit.
4. The check
Your narrow version, since the broad one needs an issue-tracker lookup from a shell script:
check "the roadmap is in the documentation nav" ...A page outside the nav is not published, and nothing else would notice it had gone. That is
how this happened.
Docs and one test script.
ste_checkclean on every user-facing document.